Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use stable, precalculated BFS for tree allocation paths #13

Merged
merged 3 commits into from
Dec 26, 2024

Conversation

dbjorge
Copy link
Contributor

@dbjorge dbjorge commented Dec 25, 2024

Resolves 3rd task in #6

Also fixed a few related pre-existing bugs I noticed while testing:

  • Previously, it treated masteries as having bidirectional graph edges, which meant you could traverse across them in clusters that have multiple notables (eg, from Path of the Warrior -> life master -> Constitution). Updated it to treat masteries as using directional graph edges to disallow this.
  • Previously, it treated all root nodes as "active", but this could result in picking a non-shortest path in some edge cases (see the new test case in tree_graph_test.go L67). Separated the "active" and "root" inputs to enable the algorithm to better handle this case.

To put together the test cases, I also implemented functionality similar to PoB 1's "developer mode" feature to show node IDs as part of their hover tooltips. I'll send that as a separate PR, but if you care to validate the test cases, it'll be much easier to do so using that PR.

Copy link

vercel bot commented Dec 25, 2024

@dbjorge is attempting to deploy a commit to the vilsol's projects Team on Vercel.

A member of the Team first needs to authorize it.

@@ -350,6 +350,21 @@ export declare namespace fwd {
WriteTo(w?: unknown): [number, Error];
}
}
export declare namespace mod {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These came in automatically when the typings got regenerated to pick up the updated calculate signature. I wasn't sure if you'd prefer for me to leave them in vs omit them from the PR, I'm fine with whichever you prefer.

Copy link

vercel bot commented Dec 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
go-pob ✅ Ready (Inspect) Visit Preview Dec 25, 2024 11:30pm

@Vilsol Vilsol merged commit 3f73ca3 into Vilsol:dev Dec 26, 2024
11 of 12 checks passed
@Vilsol Vilsol mentioned this pull request Jan 13, 2025
45 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants